xtest: add asymmetric cipher algorithm perf test #691

yuzexiyzx · 2023-08-08T06:41:23Z

Add perf test for DH,RSA,ECDH,ECDSA

jenswi-linaro

Would it make sense to refactor the *_perf TAs into a single TA instead?

jenswi-linaro · 2023-08-21T17:48:22Z

ta/asym_cipher_perf/include/user_ta_header_defines.h

+
+#define TA_UUID TA_ASYM_CIPHER_PERF_UUID
+
+#define TA_FLAGS                (TA_FLAG_USER_MODE | TA_FLAG_EXEC_DDR | \


TA_FLAG_USER_MODE and TA_FLAG_EXEC_DDR are deprecated and not needed any longer.
I'm not sure TA_FLAG_SECURE_DATA_PATH or TA_FLAG_CACHE_MAINTENANCE are needed.
We should be able to define TA_FLAGS to 0.

jenswi-linaro

I think it's time to consolidate the *_perf TAs into a single TA new perf TA.

jenswi-linaro · 2023-08-30T08:06:24Z

host/xtest/asym_cipher_perf.c

+	.flags = TEEC_MEM_INPUT | TEEC_MEM_OUTPUT
+};
+
+static void errx(const char *msg, TEEC_Result res, uint32_t *orig)


It seems that there's quite a bit of copy & paste from host/xtest/hash_perf.c or host/xtest/aes_perf.c in this file.
Duplicated code is one thing, but now we have a third copy. It's time to consolidate the code before it get even harder to maintain.

OK.I will consilidate them.What is the new TA's name?perf?

I will do that with another commit after the asyn_cipher one. Is that OK?

OK.I will consilidate them.What is the new TA's name?perf?

I suggest crypto_perf.

yuzexiyzx · 2023-09-18T07:15:03Z

@jenswi-linaro @jforissier I have modified the code according to the comments, but the CI failed. It seems that the error is not related to my change

jforissier · 2023-09-18T17:36:41Z

@jenswi-linaro @jforissier I have modified the code according to the comments, but the CI failed. It seems that the error is not related to my change

I think it is:

2023-09-15T04:12:31.8220760Z /__w/optee_test/optee_repo_qemu_v8/out-br/build/optee_test_ext-1.0/host/xtest/aes_perf.c:18:10: fatal error: ta_aes_perf.h: No such file or directory
2023-09-15T04:12:31.8221211Z    18 | #include <ta_aes_perf.h>

jforissier · 2023-09-19T10:12:41Z

It would be better to combine the apps before introducing the asymmetric ciphers...

yuzexiyzx · 2023-09-20T03:26:07Z

It would be better to combine the apps before introducing the asymmetric ciphers...
OK.Do you any have comments on the combining the apps commit? It would be easier for me to modify it before changing the commit order.

etienne-lms

Comments related to comit "xtest: add asymmetric cipher algorithm perf test".

etienne-lms · 2023-09-05T11:26:40Z

ta/asym_cipher_perf/include/ta_asym_cipher_perf.h

@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (c) 2015, Linaro Limited


Update? (2023)

etienne-lms · 2023-09-05T11:28:37Z

ta/asym_cipher_perf/include/ta_asym_cipher_perf.h

+	MODE_SIGN = 2,
+	MODE_VERIFY = 3,
+	MODE_GENKEYPAIR = 4,
+} Asym_Cipher_Mode;


prefer no typedef and use snake_case labels:

enum asym_cipher_mode { MODE_ENCRYPT = 0, MODE_DECRYPT = 1, MODE_SIGN = 2, MODE_VERIFY = 3, MODE_GENKEYPAIR = 4, }

Ditto for RSA_Cipher_Mode and RSA_Sign_Mode below.

etienne-lms · 2023-09-05T11:29:40Z

ta/asym_cipher_perf/ta_asym_cipher_perf.c

+#include <tee_internal_api.h>
+#include <tee_ta_api.h>
+#include <string.h>
+#include <trace.h>


swap the 2 above lines

etienne-lms · 2023-09-05T11:29:52Z

ta/asym_cipher_perf/ta_asym_cipher_perf.c

+ */
+
+#include <stddef.h>
+#include <utee_defines.h>


move to line 14

etienne-lms · 2023-09-05T11:33:06Z

ta/asym_cipher_perf/ta_asym_cipher_perf.c

+
+#define CHECK(res, name, action) do {			\
+		if ((res) != TEE_SUCCESS) {		\
+			printf(name ": 0x%08x", (res));	\


Use EMSG() instead of printf() for error trace messages in the TA.

Ditto for the other occurences of printf() in this TA source file.
Note: also remove \n termination character when using EMSG().

etienne-lms · 2023-09-20T12:06:12Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "                                 3: RSASSA_PKCS1_V1_5_SHA384      4:  RSASSA_PKCS1_V1_5_SHA512      5: RSASSA_PKCS1_PSS_MGF1_SHA1\n");
+	fprintf(stderr, "                                 6: RSASSA_PKCS1_PSS_MGF1_SHA224  7:  RSASSA_PKCS1_PSS_MGF1_SHA256  8: RSASSA_PKCS1_PSS_MGF1_SHA384\n");
+	fprintf(stderr, "                                 9: RSASSA_PKCS1_PSS_MGF1_SHA512\n");
+	fprintf(stderr, "  -s salt_len      only RSA SSA_PKCS1_PSS support! [%u]\n", salt_len);


-s SALT_LEN ...

etienne-lms · 2023-09-20T12:13:11Z

host/xtest/asym_cipher_perf.c

+						     &in_shm : &out_shm;
+			op.params[3].memref.size = (mode == MODE_DECRYPT) ?
+						   size : out_shm.size;
+		}


I would suggest to use a switch/case here:

... op.params[0].tmpref.buffer = buf; op.params[0].tmpref.size = blen; op.params[1].value.a = l; op.params[1].value.b = mode; switch (mode) { case MODE_ENCRYPT: op.params[2].memref.parent = &in_shm; op.params[2].memref.size = size; op.params[3].memref.parent = &out_shm; op.params[3].memref.size = out_shm.size; break; case MODE_SIGN: op.params[2].memref.parent = &hash_shm; op.params[2].memref.size = hash_shm.size; op.params[3].memref.parent = &out_shm; op.params[3].memref.size = out_shm.size; break; case MODE_DECRYPT: prepare_enc_sign(size, mode, is_random, buf, blen); op.params[2].memref.parent = &out_shm; op.params[2].memref.size = out_shm.size; op.params[3].memref.parent = &in_shm; op.params[3].memref.size = size; break; case MODE_VERIFY: prepare_enc_sign(size, mode, is_random, buf, blen); op.params[2].memref.parent = &hash_shm; op.params[2].memref.size = hash_shm.size; op.params[3].memref.parent = &out_shm; op.params[3].memref.size = out_shm.size; break; default: fprintf(stderr, "Unexpected mode value\n"); exit(1); }

etienne-lms · 2023-09-20T12:25:18Z

host/xtest/asym_cipher_perf.c

+	case RSAES_PKCS1_OAEP_SHA512:
+		return SHA512_LEN;
+	default:
+		printf("The algo[%u] is err!\n", algo);


Replace is err with is not valid
I think these error message should better be printed with fprintf(stderr, ...)

Ditto for othter is err and is error occurrences below.

host/xtest/asym_cipher_perf.c

+	int width_bytes = BITS_TO_BYTES(width_bits);
+	int salt_temp = 0;
+	int hash_len = get_hash_sign_len(rsa_algo);
+	if (hash_len == -1)


etienne-lms · 2023-09-20T12:30:10Z

ta/asym_cipher_perf/ta_asym_cipher_perf.c

+		return TEE_ERROR_BAD_PARAMETERS;
+	}
+
+	return 0;


s/0/TEE_SUCCESS/

Ditto at lines 165, 191 and 197.

etienne-lms · 2023-09-20T14:15:44Z

ta/asym_cipher_perf/include/ta_asym_cipher_perf.h

+ */
+
+#ifndef TA_ASYM_CIPHER_PERF_H
+#define TA_ASYM_CIPHER_PERF_H


By the way, this TA does more than just cipher (sign/verify). Rename to ta_asymm_crypto_perf.h?

etienne-lms · 2023-09-22T15:48:26Z

host/xtest/asym_cipher_perf.c

+
+#include "crypto_common.h"
+#include "xtest_helpers.h"
+#include <utee_defines.h>


move this line between line 20 and lien 21?

etienne-lms · 2023-09-22T15:49:54Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, " [-t tee_type] [-m mode] [-n LOOP] [-r|--no-inited] [-d WIDTH_BITS]");
+	fprintf(stderr, " [-k SIZE] [-a rsa_algo] [-s salt_len] [-v [-v]] [-w SEC]");
+	fprintf(stderr, "\n");
+	fprintf(stderr, "Asymmetric Cipher performance testing tool for OP-TEE\n");


remove "Cipher" (here and at few other places) since this is not only about ciphering.

etienne-lms · 2023-09-22T15:51:03Z

host/xtest/asym_cipher_perf.c

+						     &in_shm : &out_shm;
+			op.params[3].memref.size = (mode == MODE_DECRYPT) ?
+						   size : out_shm.size;
+		}


etienne-lms · 2023-09-22T15:53:23Z

ta/crypto_perf/ta_crypto_perf.c

+	ap = (const struct attr_packed *)(const void *)(buf + num_attrs_size);
+
+	if (num_attrs >= 0) {
+		size_t n;


etienne-lms · 2023-09-22T15:53:32Z

ta/crypto_perf/ta_crypto_perf.c

+		if (!a)
+			return TEE_ERROR_OUT_OF_MEMORY;
+		for (n = 0; n < num_attrs; n++) {
+			uintptr_t p;


etienne-lms · 2023-09-22T15:57:38Z

ta/crypto_perf/ta_crypto_perf.c

+}
+
+TEE_Result cmd_asym_process_rsa_ecc(uint32_t param_types,
+			       TEE_Param params[TEE_NUM_PARAMS])


indentation

etienne-lms · 2023-09-22T15:57:54Z

ta/crypto_perf/ta_crypto_perf.c

+						   TEE_PARAM_TYPE_MEMREF_OUTPUT);
+	TEE_Attribute *attrs = NULL;
+	uint32_t attr_count = 0;
+


remove this empty line

etienne-lms · 2023-09-22T15:59:19Z

ta/crypto_perf/ta_crypto_perf.c

+					  &crypto_obj);
+	CHECK(res, "TEE_AllocateTransientObject", return res;);
+
+	return res;


at this stage we're always successful. Prefer explicit return TEE_SUCCESS.

etienne-lms · 2023-09-22T16:00:10Z

ta/crypto_perf/ta_crypto_perf.c

+
+	TEE_FreeOperation(hash_op);
+
+	return res;


return TEE_SUCCESS

etienne-lms · 2023-09-22T16:01:28Z

ta/crypto_perf/ta_crypto_perf.c

+					       &params[1].memref.size);
+
+	TEE_Free(attrs);
+	CHECK(res, "TEE_AsymmetricEncrypt", return res;);


Fix trace message, e.g.:

if (mode == MODE_DECRYPT) CHECK(res, "TEE_AsymmetricEncrypt", return res;); else CHECK(res, "TEE_AsymmetricSignDigest", return res;);

yuzexiyzx · 2023-10-07T03:00:24Z

@jforissier @etienne-lms I have modified the code according to the comments.

jenswi-linaro · 2023-10-09T07:09:43Z

ta/crypto_perf/include/user_ta_header_defines.h

-#endif
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (c) 2023, Linaro Limited


I believe the copyright from 2015 should be kept since this file has only been edited.

jenswi-linaro · 2023-10-09T07:10:09Z

ta/crypto_perf/include/user_ta_header_defines.h

+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (c) 2023, Linaro Limited
+ * All rights reserved.


Drop this please, we don't use that any longer.

jenswi-linaro · 2023-10-09T07:18:35Z

ta/crypto_perf/ta_crypto_perf.c

+		objectType = TEE_TYPE_AES;
+		use_iv = 1;
+		break;
+	case TA_SM4_ECB:


SM4 support looks new, please mention that in the commit message.

jforissier

Reviewing commit "xtest: combine aes_perf and hash_perf into crypto_perf".

With my comments addressed:
Acked-by: Jerome Forissier <[email protected]>

jforissier · 2023-10-10T08:03:01Z

ta/crypto_perf/include/ta_crypto_perf.h

+#define TA_CIPHER_PERF_CMD_PREPARE_KEY			0
+#define TA_CIPHER_PERF_CMD_PROCESS			1
+#define TA_CIPHER_PERF_CMD_PROCESS_SDP			2
+#define TA_HASH_PERF_CMD_PREPARE_OP			3
+#define TA_HASH_PERF_CMD_PROCESS			4


Please keep a common TA_CRYPTO_PERF_CMD_ prefix, to be consistent with TA_CRYPTO_PERF_UUID.

#define TA_CRYPTO_PERF_CMD_CIPHER_PREPARE_KEY 0 #define TA_CRYPTO_PERF_CMD_CIPHER_PROCESS 1 #define TA_CRYPTO_PERF_CMD_CIPHER_PROCESS_SDP 2 #define TA_CRYPTO_PERF_CMD_HASH_PREPARE_OP 3 #define TA_CRYPTO_PERF_CMD_HASH_PROCESS 4 ...

jforissier · 2023-10-10T08:04:53Z

ta/crypto_perf/Android.mk

-include $(BUILD_OPTEE_MK)
+LOCAL_PATH := $(call my-dir)
+
+local_module := e626662e-c0e2-485c-b8c8-09fbce6edf3d.ta


I believe the UUID should be changed. The new TA is not compatible with the old one, it accepts a different list of commands.

etienne-lms · 2023-09-25T07:01:36Z

ta/CMakeLists.txt

@@ -13,7 +12,7 @@ target_include_directories(${PROJECT_NAME}
 	INTERFACE os_test/include
 	INTERFACE rpc_test/include
 	INTERFACE sdp_basic/include
-	INTERFACE hash_perf/include
+	INTERFACE crypto_perf/include


nitpicking: would be better placed below line 10 (alphabetical order)

etienne-lms · 2023-10-10T07:57:54Z

ta/crypto_perf/ta_crypto_perf.c

+#include <tee_internal_api_extensions.h>
+#include <tee_internal_api.h>
+#include <tee_ta_api.h>
+#include <string.h>


could you move this at top

ta/crypto_perf/include/ta_crypto_perf.h

@@ -72,5 +77,54 @@
 #define SHA256_LEN 32
 #define SHA384_LEN 48
 #define SHA512_LEN 64
+#define WIDTH_BITS_25519 256
+


ta/crypto_perf/ta_crypto_perf.c

@@ -486,9 +488,479 @@ TEE_Result cmd_hash_prepare_op(uint32_t param_types, TEE_Param params[4])
 	}
 	return TEE_SUCCESS;
 }
+struct attr_packed {


etienne-lms · 2023-10-10T08:10:58Z

ta/crypto_perf/ta_crypto_perf.c

+
+	if (IS_ALIGNED_WITH_TYPE(buf, uint32_t) || blen < num_attrs_size)
+		return TEE_ERROR_BAD_PARAMETERS;
+	num_attrs = *(uint32_t *) (void *)buf;


please remove the space char. We don't use space char when casting: num_attrs = *(uint32_t *)(void *)buf;

ta/crypto_perf/ta_crypto_perf.c

+						   TEE_PARAM_TYPE_MEMREF_INPUT,
+						   TEE_PARAM_TYPE_MEMREF_INOUT,
+						   TEE_PARAM_TYPE_NONE);
+	if (param_types != exp_param_types)


etienne-lms · 2023-10-10T09:16:05Z

By the way, commit "xtest: add asymmetric cipher and SM4 algorithm perf test" no more implements the SM4 part. Could you update the commit message.

jenswi-linaro · 2023-10-16T09:01:13Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "  -d WIDTH_BITS    ECC:the width_bits only support 192/224/256/384/521 [%u]\n", width_bits);
+	fprintf(stderr, "                   DH: width_bits <= 2048, RSA: width_bits <= 4096 [%u]\n", width_bits);
+	fprintf(stderr, "  -a ALGO          Crypto algorithm tested when TYPE is 2(RSA)\n");
+	fprintf(stderr, "                   EN/DE:        0: RSA_NOPAD                     1:  RSAES_PKCS1_V1_5              2: RSAES_PKCS1_OAEP_SHA1\n");


These lines become too long in the usage() print. The width of a terminal is 80 columns.

jenswi-linaro · 2023-10-16T09:05:47Z

host/xtest/asym_cipher_perf.c

+	int warmup = CRYPTO_DEF_WARMUP;
+	int mode = MODE_GENKEYPAIR;
+	int width_bits = 1024;
+	uint32_t tee_type = 0;


It would be more convenient to have a valid default value.

jenswi-linaro · 2023-10-16T09:06:27Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "\n");
+	fprintf(stderr, "Options:\n");
+	fprintf(stderr, "  -h|--help        Print this help and exit\n");
+	fprintf(stderr, "  -t TYPE          Test type for asymmetric algo [%u]\n", tee_type);


This parameter should not be optional if the default value isn't usable.

jenswi-linaro · 2023-10-16T09:10:29Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "Options:\n");
+	fprintf(stderr, "  -h|--help        Print this help and exit\n");
+	fprintf(stderr, "  -t TYPE          Test type for asymmetric algo [%u]\n", tee_type);
+	fprintf(stderr, "                   1:DH    2:RSA    3:ECDSA    4:ECDH    5:X25519\n");


I'd rather take these parameters as strings. When this is used in a script or a saved sequence of commands it's easier to see what's tested. Same for the -m and -a parameters.

May I keep this way to -a paramter,since there are too many cases in -a parameter.What's more, it is different for meaning of num when mode is ENC/DEC or SIGN/VERIFY

Looping over const char *asym_algo[] = { "DH", "RSA", "ECDSA", "ECDH", "X25519" }; to determine the index and use that can't be too hard.

DH,RSA……are -t parameters.I mean -a parameters，such as RSAES_PKCS1_V1_5，RSAES_PKCS1_OAEP_SHA256 and so on.

The greater reason to use the same approach there.

@yuzexiyzx you did not address this comment

etienne-lms

I think it would help to have 3 commits here. 1 for combining AES/hashes tests TA a,d applets, 1 for adding SM4 tests and the last one for asymm crypto tests. Sorry for such a late comment.

host/xtest/xtest_main.c

@@ -103,6 +103,7 @@ void usage(char *program)
 	printf("\t--sha-perf [opts]  Deprecated, same as --hash-perf\n");
 	printf("\t--hash-perf [opts] Hash performance testing tool (-h for usage)\n");
 	printf("\t--aes-perf [opts]  AES performance testing tool (-h for usage)\n");
+	printf("\t--asym-cipher-perf [opts]  Aysmmetric cipher performance testing tool (-h for usage)\n");


etienne-lms · 2023-10-17T10:17:53Z

host/xtest/asym_cipher_perf.c

+#include <tee_client_api_extensions.h>
+#include <time.h>
+#include <utee_defines.h>
+#include <unistd.h>


swap the 2 above lines.

etienne-lms · 2023-10-17T10:19:16Z

host/xtest/asym_cipher_perf.c

+struct statistics {
+	int n;
+	double m;
+	double M2;


prefer lower case m2 (use uppercase for macros).

yuzexiyzx · 2023-10-30T03:16:15Z

SM4 need more modification on CA side,so I will do this later in other pull request.SM4 will not be added here.

jforissier

xtest: combine aes and hash into crypto_perf

s/aes/AES/

There is quite a bit of copy in these files.So, I cosolidate the

consolidate

code to make it easier to maintain.In addition, SM4 algorithm is
added.

SM4 is not added, please remove.

jforissier · 2023-10-30T09:25:26Z

ta/crypto_perf/include/ta_crypto_perf.h

@@ -18,6 +18,12 @@
 #define TA_CRYPTO_PERF_CMD_CIPHER_PROCESS_SDP			2
 #define TA_CRYPTO_PERF_CMD_HASH_PREPARE_OP			3
 #define TA_CRYPTO_PERF_CMD_HASH_PROCESS				4
+#define TA_CRYPTO_PERF_CMD_ASYM_CIPHER_PREPARE_OBJ		5
+#define TA_CRYPTO_PERF_CMD_ASYM_CIPHER_PREPARE_HASH		6
+#define TA_CRYPTO_CMD_ASYM_CIPHER_PREPARE_KEYPAIR		7


TA_CRYPTO_PERF_CMD_ASYM_CIPHER_PREPARE_KEYPAIR

etienne-lms

For commit "xtest: combine AES and hash into crypto_perf":
Acked-by: Etienne Carriere <[email protected]> with the 2 issues fixed.

etienne-lms · 2023-10-30T16:34:29Z

ta/crypto_perf/include/ta_crypto_perf.h

+#define TA_HMAC_SHA512	10
+#define TA_HMAC_SM3	11
+
+#define PKCS_V1_5_MIN 11


added macros from line 56 tp 68 belongs to commit "xtest: add asymmetric cipher perf test".

etienne-lms · 2023-10-30T16:37:00Z

ta/crypto_perf/ta_entry.c

+		return cmd_hash_prepare_op(nParamTypes, pParams);
+	case TA_CRYPTO_PERF_CMD_HASH_PROCESS:
+		return cmd_hash_process(nParamTypes, pParams);
+	case TA_CRYPTO_PERF_CMD_ASYM_CIPHER_PROCESS_GEN_KEYPAIR:


addes cases from line 72 to line 82 belongs the the next commit in the series.

etienne-lms

Comments for commit "xtest: add asymmetric cipher perf test".

etienne-lms · 2023-10-30T16:39:34Z

Android.mk

@@ -53,6 +53,7 @@ srcs +=	adbg/src/adbg_case.c \
 	regression_8000.c \
 	regression_8100.c \
 	hash_perf.c \
+	asym_cipher_perf.c \


nitpicking; would be better right below line 45 to follow alphabetical order.

etienne-lms · 2023-10-30T16:48:35Z

ta/crypto_perf/ta_crypto_perf.c

+	return res;
+}
+
+static TEE_Result get_keypair_type(uint32_t value)


This function wrongly mixes TEE_Result values and TEE_TYPE_xxx values.
You could use TEE_TYPE_ILLEGAL_VALUE as error case return value, and change this function to return a uint32_t value.

etienne-lms · 2023-10-30T16:49:47Z

ta/crypto_perf/ta_crypto_perf.c

+TEE_Result cmd_asym_prepare_obj(uint32_t param_types, TEE_Param params[4])
+{
+	TEE_Result res = TEE_ERROR_GENERIC;
+	uint32_t tee_type;


= TEE_TYPE_ILLEGAL_VALUE

etienne-lms · 2023-10-30T16:51:52Z

ta/crypto_perf/ta_crypto_perf.c

+	res = TEE_DigestDoFinal(hash_op, params[1].memref.buffer,
+				params[1].memref.size, params[2].memref.buffer,
+				&params[2].memref.size);
+	CHECK(res, "TEE_DigestDoFinal", return res;);


call TEE_FreeOperation(hash_op); before line 882.

etienne-lms · 2023-10-30T16:55:45Z

host/xtest/asym_cipher_perf.c

+	s->m += delta / s->n;
+	s->m2 += delta * (x - s->m);
+	if (!s->initialized) {
+		s->min = s->max = x;


prefer split over 2 lines:

s->min = x; s->max = x;

etienne-lms · 2023-10-30T16:59:08Z

host/xtest/asym_cipher_perf.c

+	if (tee_type == ECDSA || tee_type == ECDH) {
+		curve_id = get_curve_id(width_bits);
+		if (curve_id == TEE_ERROR_BAD_PARAMETERS)
+			return TEE_ERROR_BAD_PARAMETERS;


return -1;

etienne-lms · 2023-10-30T17:03:13Z

host/xtest/asym_cipher_perf.c

+
+static uint32_t get_curve_id(uint32_t width_bits)
+{
+	uint32_t curve_id = TEE_ERROR_BAD_PARAMETERS;


use TEE_CRYPTO_ELEMENT_NONE that is the ID defined in the spec for when no curve element ID is applicable.
Definitly not TEE_ERROR_BAD_PARAMETERS that is a TEE_Result reserved ID.

etienne-lms · 2023-10-30T17:03:48Z

host/xtest/asym_cipher_perf.c

+		curve_id = TEE_ECC_CURVE_NIST_P521;
+		break;
+	default:
+		printf("ECC curve is not support!\n");


"... is not supported\n"

etienne-lms · 2023-10-30T17:07:04Z

host/xtest/asym_cipher_perf.c

+	free_shm();
+	free(buf);
+	close_ta();
+	return res;


At this stage, the perf test did not fail. Prefer return 0

etienne-lms · 2023-10-30T17:10:09Z

host/xtest/asym_cipher_perf.c

+	}
+
+	res = pack_attrs(params, param_count, &buf, &blen);
+	CHECK(res, "pack_attrs", return res;);


s/return res;/return -1;/ for consistency in return codes.
Ditto at line 458.

jenswi-linaro · 2023-11-03T14:53:08Z

host/xtest/asym_cipher_perf.c

+		} else if (mode == MODE_SIGN || mode == MODE_VERIFY) {
+			if (check_rsa_hash_params(rsa_algo, width_bits, size,
+						  salt_len))
+			return -1;


Indentation

jenswi-linaro · 2023-11-03T15:05:06Z

host/xtest/asym_cipher_perf.c

+{
+	fprintf(stderr, "Usage: %s [-h]\n", progname);
+	fprintf(stderr, "Usage: %s [-t] [-m] [-k SIZE]", progname);
+	fprintf(stderr, " [-t tee_type] [-m mode] [-n LOOP] [-r|--no-inited] [-d WIDTH_BITS]");


The name tee_type is confusing, please find a better name. Perhaps algo_class or main_algo?

jenswi-linaro · 2023-11-03T15:07:28Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "Options:\n");
+	fprintf(stderr, "  -h|--help        Print this help and exit\n");
+	fprintf(stderr, "  -t TYPE          Test type for asymmetric algo [%u]\n", tee_type);
+	fprintf(stderr, "                   1:DH 2:RSA 3:ECDSA 4:ECDH 5:X25519\n");


The numbers mean nothing now, please remove them. Same below.

jenswi-linaro · 2023-11-03T15:08:08Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "  -d WIDTH_BITS    ECC:the width_bits only support 192/224/256/384/521 [%u]\n", width_bits);
+	fprintf(stderr, "                   DH: width_bits <= 2048, RSA: width_bits <= 4096 [%u]\n", width_bits);
+	fprintf(stderr, "  -a ALGO          Crypto algorithm tested when TYPE is 2(RSA)\n");
+	fprintf(stderr, "  EN/DE: 0:RSA_NOPAD 1:RSAES_PKCS1_V1_5  2:RSAES_PKCS1_OAEP_SHA1\n");


ENC/DEC to match the -m MODE above.

jenswi-linaro · 2023-11-03T15:08:29Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "  EN/DE: 0:RSA_NOPAD 1:RSAES_PKCS1_V1_5  2:RSAES_PKCS1_OAEP_SHA1\n");
+	fprintf(stderr, "         3:RSAES_PKCS1_OAEP_SHA224 4:RSAES_PKCS1_OAEP_SHA256\n");
+	fprintf(stderr, "         5:RSAES_PKCS1_OAEP_SHA384 6:RSAES_PKCS1_OAEP_SHA512\n");
+	fprintf(stderr, "  SIGN/VERSIGN: 0:RSASSA_PKCS1_V1_5_SHA1 1:RSASSA_PKCS1_V1_5_SHA224\n");


SIGN/VERIFY

jenswi-linaro · 2023-11-03T15:11:29Z

host/xtest/asym_cipher_perf.c

+{
+	uint32_t rsa_cipher_mode = RSA_NOPAD;
+
+	if (!strcasecmp(argv, "RSA_NOPAD"))


I don't suppose it's harmful, but why should we ignore the case?

jenswi-linaro · 2023-11-03T15:15:53Z

ta/crypto_perf/include/ta_crypto_perf.h

@@ -18,6 +18,12 @@
 #define TA_CRYPTO_PERF_CMD_CIPHER_PROCESS_SDP			2
 #define TA_CRYPTO_PERF_CMD_HASH_PREPARE_OP			3
 #define TA_CRYPTO_PERF_CMD_HASH_PROCESS				4
+#define TA_CRYPTO_PERF_CMD_ASYM_CIPHER_PREPARE_OBJ		5


Please remove the "carriage ret" characters that it seems your editor has added in all patches of this PR.

ta/crypto_perf/ta_crypto_perf.c

+			EMSG("ECDSA error mode");
+			res = TEE_ERROR_BAD_PARAMETERS;
+		}
+	}


etienne-lms · 2023-11-06T05:53:17Z

ta/crypto_perf/ta_crypto_perf.c

+		return TEE_ERROR_BAD_PARAMETERS;
+
+	tee_type = get_keypair_type(params[0].value.a);
+	if (tee_type == TEE_ERROR_BAD_PARAMETERS)


if (tee_type == TEE_TYPE_ILLEGAL_VALUE)

etienne-lms · 2023-11-06T05:57:07Z

ta/crypto_perf/ta_crypto_perf.c

+	else if (mode == MODE_SIGN)
+		do_asym = TEE_AsymmetricSignDigest;
+
+	if (mode == MODE_VERIFY) {


It would be more consistent to use

else if (mode == MODE_VERIFY) do_asym = TEE_AsymmetricVerifyDigest;

above and remove the below specific loop.

etienne-lms · 2023-11-06T06:00:10Z

ta/crypto_perf/ta_crypto_perf.c

+				params[2].memref.buffer, params[2].memref.size,
+				params[3].memref.buffer, &params[3].memref.size);
+
+			CHECK(res, "TEE_AsymmetricEncrypt", goto out;);


for consistency and simplicity: CHECK(res, "TEE proccessing failed", goto out;);

etienne-lms · 2023-11-08T08:22:49Z

host/xtest/Makefile

@@ -68,6 +68,7 @@ srcs +=	adbg/src/adbg_case.c \
 	regression_8000.c \
 	regression_8100.c \
 	hash_perf.c \
+	asym_cipher_perf.c \


nitpicking: prefer between aes_perf.c \ and benchmark_1000.c \ to follow the alphabetical order.

etienne-lms · 2023-11-08T08:24:36Z

host/xtest/asym_cipher_perf.c

+	if (orig)
+		fprintf(stderr, " (orig=%d)", (int)*orig);
+	fprintf(stderr, "\n");
+	exit (1);


remove the space char

etienne-lms · 2023-11-08T08:28:14Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "\n");
+	fprintf(stderr, "Options:\n");
+	fprintf(stderr, "  -h|--help        Print this help and exit\n");
+	fprintf(stderr, "  -t TYPE          Test type for asymmetric algo [%u]\n", main_algo);


I think there should not be a default value for algo type. Also main_algo is a numerical value whereas the option argument is expected to be a string: DH, RSA, ...
All in one, remove [%u] here IMHO.

etienne-lms · 2023-11-08T08:28:33Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "  -h|--help        Print this help and exit\n");
+	fprintf(stderr, "  -t TYPE          Test type for asymmetric algo [%u]\n", main_algo);
+	fprintf(stderr, "                   DH, RSA, ECDSA, ECDH, X25519\n");
+	fprintf(stderr, "  -m MODE          Test asymmetric mode [%u]\n", mode);


ditto about [%u]

etienne-lms · 2023-11-08T08:29:14Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "  -k SIZE          Plaintext Length [%u]\n", size);
+	fprintf(stderr, "  -d WIDTH_BITS    ECC:the width_bits only support 192/224/256/384/521 [%u]\n", width_bits);
+	fprintf(stderr, "                   DH: width_bits <= 2048, RSA: width_bits <= 4096 [%u]\n", width_bits);
+	fprintf(stderr, "  -a ALGO          Crypto algorithm tested when TYPE is 2(RSA)\n");


s/2(RSA)/RSA/

etienne-lms · 2023-11-08T08:46:03Z

host/xtest/asym_cipher_perf.c

+	int width_bytes = BITS_TO_BYTES(width_bits);
+	int hash_len = 0;
+
+	if (rsa_algo == 0) {


s/0/RSA_NOPAD/

host/xtest/asym_cipher_perf.c

+			fprintf(stderr, "The size or algo is not valid!\n");
+			return -1;
+		}
+	}


etienne-lms · 2023-11-08T08:47:09Z

host/xtest/asym_cipher_perf.c

+		}
+	}
+
+	return TEE_SUCCESS;


s/TEE_SUCCESS/0/

etienne-lms · 2023-11-08T08:47:20Z

host/xtest/asym_cipher_perf.c

+		}
+	}
+
+	return TEE_SUCCESS;


s/TEE_SUCCESS/0/

etienne-lms · 2023-11-08T09:26:33Z

ta/crypto_perf/ta_crypto_perf.c

+	if (mode == MODE_DECRYPT)
+		res = TEE_AsymmetricEncrypt(crypto_op_enc_sign, NULL, 0,
+				params[0].memref.buffer, params[0].memref.size,
+				params[1].memref.buffer, &params[1].memref.size);


indentation

res = TEE_AsymmetricEncrypt(crypto_op_enc_sign, NULL, 0, params[0].memref.buffer, params[0].memref.size, params[1].memref.buffer, &params[1].memref.size);

etienne-lms

For commit "xtest: combine AES and hash into crypto_perf":

 Signed-off-by: Zexi Yu <[email protected]>
 Acked-by: Jerome Forissier <[email protected]>
-          Etienne Carriere <[email protected]>
+Acked-by: Etienne Carriere <[email protected]>

There is quite a bit of copy in these files.So, I cosonlidate the code to make it easier to maintain. Signed-off-by: Zexi Yu <[email protected]> Acked-by: Jerome Forissier <[email protected]> Acked-by: Etienne Carriere <[email protected]>

etienne-lms

This P-R is not far from being fine. Thanks for your patience. Some remaining issues to fix and few nitpicking comments to address.

Have a look at the CI test

etienne-lms · 2023-11-09T05:29:06Z

ta/crypto_perf/ta_crypto_perf.c

+	case X25519:
+		return TEE_TYPE_X25519_KEYPAIR;
+	default:
+		EMSG("The algo[%u] is not valid", algo);


the variable name is value
Try to build with CFG_TEE_TA_LOG_LEVEL=0 then CFG_TEE_TA_LOG_LEVEL=4 to check debug traces syntax.

etienne-lms · 2023-11-09T05:31:17Z

ta/crypto_perf/ta_crypto_perf.c

+}
+
+TEE_Result cmd_asym_process_keypair(uint32_t param_types,
+			       TEE_Param params[TEE_NUM_PARAMS])


indentation

etienne-lms · 2023-11-09T05:34:04Z

ta/crypto_perf/ta_crypto_perf.c

+	return res;
+}
+
+TEE_Result cmd_asym_prepare_obj(uint32_t param_types, TEE_Param params[4])


s/4/TEE_NUM_PARAMS/ for consistency with other functions.

Ditto for cmd_asym_prepare_keypair() and cmd_asym_prepare_hash().

etienne-lms · 2023-11-09T05:38:05Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "\n");
+	fprintf(stderr, "Options:\n");
+	fprintf(stderr, "  -h|--help        Print this help and exit\n");
+	fprintf(stderr, "  -t TYPE          Test type for asymmetric algo\n", main_algo);


remove main_algo argument since no more printed.

same comment at line 141.

etienne-lms · 2023-11-09T05:41:02Z

host/xtest/asym_cipher_perf.c

+
+	out_shm.size = op.params[1].memref.size;
+
+	return;


nitpicking: for consistency with the other functions implementation:

res = TEEC_InvokeCommand(&sess, cmd, &op, &ret_origin); - check_res(res, "TEEC_InvokeCommand", &ret_origin); - out_shm.size = op.params[1].memref.size; - - return; }

etienne-lms · 2023-11-09T05:46:46Z

host/xtest/asym_cipher_perf.c

+			/* Make sure the N > M */
+			((unsigned char *)(in_shm.buffer))[0] = 0x00;
+			/* Avoid the problem that the last encryption result is
+			less than the plaintext. */


/* * Avoid the problem that the last encryption result is * less than the plaintext. */

replace less with shorter?

etienne-lms · 2023-11-09T05:49:11Z

host/xtest/asym_cipher_perf.c

+	return -1;
+}
+
+static int get_hash_sign_len(uint32_t algo)


I think get_hash_sign_len() and get_hash_sign_len() could be merged into a single get_hash_len() function.

etienne-lms · 2023-11-09T05:50:52Z

host/xtest/asym_cipher_perf.c

+			fprintf(stderr, "The size or algo is not valid!\n");
+			return -1;
+		}
+	}


} else { return -1; }

etienne-lms · 2023-11-09T05:56:24Z

host/xtest/asym_cipher_perf.c

+	if (main_algo == RSA) {
+		if (mode == MODE_ENCRYPT || mode == MODE_DECRYPT) {
+			if (check_rsa_cipher_params(rsa_algo, width_bits, size))
+				return -1;


also call USAGE();

ditto at line 842

etienne-lms · 2023-11-09T06:01:55Z

host/xtest/asym_cipher_perf.c

+
+	if (main_algo == ECDSA || main_algo == ECDH) {
+		curve_id = get_curve_id(width_bits);
+		if (curve_id == TEE_CRYPTO_ELEMENT_NULL)


TEE_CRYPTO_ELEMENT_NONE

Add perf test for DH, RSA, ECDH, ECDSA algorithm Signed-off-by: Zexi Yu <[email protected]>

jforissier · 2023-10-30T15:14:33Z

Android.mk

@@ -53,6 +53,7 @@ srcs +=	adbg/src/adbg_case.c \
 	regression_8000.c \
 	regression_8100.c \
 	hash_perf.c \
+	asym_cipher_perf.c \


Let's use alphabetical order please since the list is mostly sorted already (hash_perf.c needs to move though)

jforissier · 2023-10-30T15:14:56Z

host/xtest/CMakeLists.txt

@@ -57,6 +57,7 @@ set (SRC
 	regression_8000.c
 	regression_8100.c
 	hash_perf.c
+	asym_cipher_perf.c


jforissier · 2023-10-30T15:15:02Z

host/xtest/Makefile

@@ -68,6 +68,7 @@ srcs +=	adbg/src/adbg_case.c \
 	regression_8000.c \
 	regression_8100.c \
 	hash_perf.c \
+	asym_cipher_perf.c \


jforissier · 2023-11-03T08:55:12Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "Options:\n");
+	fprintf(stderr, "  -h|--help        Print this help and exit\n");
+	fprintf(stderr, "  -t TYPE          Test type for asymmetric algo [%u]\n", tee_type);
+	fprintf(stderr, "                   1:DH    2:RSA    3:ECDSA    4:ECDH    5:X25519\n");


@yuzexiyzx you did not address this comment

jforissier · 2023-11-10T15:58:27Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "  		    RSA_NOPAD, RSAES_PKCS1_V1_5, RSAES_PKCS1_OAEP_SHA1\n");
+	fprintf(stderr, "         	    RSAES_PKCS1_OAEP_SHA224, RSAES_PKCS1_OAEP_SHA256\n");
+	fprintf(stderr, "         	    RSAES_PKCS1_OAEP_SHA384, RSAES_PKCS1_OAEP_SHA512\n");
+	fprintf(stderr, "  		    RSASSA_PKCS1_V1_5_SHA1, RSASSA_PKCS1_V1_5_SHA224\n");


Replace tabs with 8 spaces

jforissier · 2023-11-10T15:59:00Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "                   RSASSA_PKCS1_V1_5_SHA256, RSASSA_PKCS1_V1_5_SHA384\n");
+	fprintf(stderr, "                   RSASSA_PKCS1_V1_5_SHA512, RSASSA_PKCS1_PSS_MGF1_SHA1\n");
+	fprintf(stderr, "                   RSASSA_PKCS1_PSS_MGF1_SHA224, RSASSA_PKCS1_PSS_MGF1_SHA256\n");
+	fprintf(stderr, "                   RSASSA_PKCS1_PSS_MGF1_SHA384, RSASSA_PKCS1_PSS_MGF1_SHA512\n");


Please add the default algorithm in square brackets like for other options.

jforissier · 2023-11-10T16:02:07Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "                   RSASSA_PKCS1_V1_5_SHA512, RSASSA_PKCS1_PSS_MGF1_SHA1\n");
+	fprintf(stderr, "                   RSASSA_PKCS1_PSS_MGF1_SHA224, RSASSA_PKCS1_PSS_MGF1_SHA256\n");
+	fprintf(stderr, "                   RSASSA_PKCS1_PSS_MGF1_SHA384, RSASSA_PKCS1_PSS_MGF1_SHA512\n");
+	fprintf(stderr, "  -s SALT_LEN      only RSA SSA_PKCS1_PSS support! [%u]\n", salt_len);


" -s SALT_LEN Salt length in bytes (only when ALGO is one of RSASSA_PKCS1_PSS_*) [%u]\n", salt_len

jforissier · 2023-11-10T16:03:13Z

host/xtest/asym_cipher_perf.c

+	fprintf(stderr, "  -k SIZE          Plaintext Length [%u]\n", size);
+	fprintf(stderr, "  -d WIDTH_BITS    ECC:the width_bits only support 192/224/256/384/521 [%u]\n", width_bits);
+	fprintf(stderr, "                   DH: width_bits <= 2048, RSA: width_bits <= 4096 [%u]\n", width_bits);
+	fprintf(stderr, "  -a ALGO          Crypto algorithm tested when TYPE is RSA\n");


I tried providing unsupported strings (-a XYZ) and also give -a when -t is not RSA, and I did not receive proper error messages.

jforissier · 2023-11-10T16:10:26Z

@yuzexiyzx please address comments, rebase onto latest master, and submit another PR. This one is getting too big, I often get error messages from GitHub when loading the page. Thanks.

etienne-lms · 2023-11-13T09:38:11Z

For the record: superseded by #706

jenswi-linaro reviewed Aug 21, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch from db6401e to 2be5b36 Compare August 29, 2023 04:02

jenswi-linaro reviewed Aug 30, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch 3 times, most recently from d7350b2 to 8639913 Compare September 19, 2023 07:54

etienne-lms reviewed Sep 20, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch 3 times, most recently from 3fcbed6 to 0bcf34f Compare September 22, 2023 06:23

etienne-lms reviewed Sep 22, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch from 0bcf34f to 80a90fc Compare September 26, 2023 01:55

jenswi-linaro reviewed Oct 9, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch from 80a90fc to 904b680 Compare October 10, 2023 03:23

jforissier reviewed Oct 10, 2023

View reviewed changes

etienne-lms reviewed Oct 10, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch from 904b680 to ed37dae Compare October 16, 2023 07:16

jenswi-linaro reviewed Oct 16, 2023

View reviewed changes

etienne-lms reviewed Oct 18, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch 2 times, most recently from 26c4f82 to 7aa2296 Compare October 30, 2023 04:20

jforissier reviewed Oct 30, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch 2 times, most recently from 9459096 to 0ff0d37 Compare October 30, 2023 11:48

etienne-lms reviewed Oct 30, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch from 0ff0d37 to b48b984 Compare November 3, 2023 03:09

jenswi-linaro reviewed Nov 3, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch from b48b984 to 671c27b Compare November 6, 2023 03:57

etienne-lms reviewed Nov 6, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch from 671c27b to ae659fe Compare November 6, 2023 11:20

etienne-lms reviewed Nov 8, 2023

View reviewed changes

xtest: combine AES and hash into crypto_perf

aa7454b

There is quite a bit of copy in these files.So, I cosonlidate the code to make it easier to maintain. Signed-off-by: Zexi Yu <[email protected]> Acked-by: Jerome Forissier <[email protected]> Acked-by: Etienne Carriere <[email protected]>

yuzexiyzx force-pushed the master branch from ae659fe to 910e7c5 Compare November 9, 2023 02:31

etienne-lms suggested changes Nov 9, 2023

View reviewed changes

yuzexiyzx force-pushed the master branch 2 times, most recently from 66f9d70 to 1f353a1 Compare November 9, 2023 08:28

xtest: add asymmetric cipher perf test

a15f708

Add perf test for DH, RSA, ECDH, ECDSA algorithm Signed-off-by: Zexi Yu <[email protected]>

yuzexiyzx force-pushed the master branch from 1f353a1 to a15f708 Compare November 10, 2023 01:24

jforissier assigned jforissier and unassigned jforissier Nov 10, 2023

jforissier reviewed Nov 10, 2023

View reviewed changes

yuzexiyzx closed this Nov 13, 2023

yuzexiyzx mentioned this pull request Dec 7, 2023

xtest: add asymmetric cipher perf test #714

Merged


		#define TA_UUID TA_ASYM_CIPHER_PERF_UUID

		#define TA_FLAGS (TA_FLAG_USER_MODE \| TA_FLAG_EXEC_DDR \| \

xtest: add asymmetric cipher algorithm perf test #691

xtest: add asymmetric cipher algorithm perf test #691

Conversation

yuzexiyzx commented Aug 8, 2023 • edited Loading

jenswi-linaro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenswi-linaro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuzexiyzx commented Sep 18, 2023

jforissier commented Sep 18, 2023

jforissier commented Sep 19, 2023

yuzexiyzx commented Sep 20, 2023

etienne-lms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuzexiyzx commented Oct 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jforissier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

This comment was marked as resolved.

etienne-lms commented Oct 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etienne-lms left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuzexiyzx commented Oct 30, 2023

jforissier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etienne-lms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etienne-lms left a comment

yuzexiyzx commented Aug 8, 2023 •

edited

Loading